Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedFailed to post review comments. Configuration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (22)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧬 Code graph analysis (2)applications/datamanager/src/datamanager/main.py (1)
applications/portfoliomanager/src/portfoliomanager/main.py (1)
🪛 LanguageToolCLAUDE.md[grammar] ~7-~7: There might be a mistake here. (QB_NEW_EN) [grammar] ~8-~8: There might be a mistake here. (QB_NEW_EN) [grammar] ~9-~9: There might be a mistake here. (QB_NEW_EN) [grammar] ~10-~10: There might be a mistake here. (QB_NEW_EN) [grammar] ~11-~11: There might be a mistake here. (QB_NEW_EN) [grammar] ~12-~12: There might be a mistake here. (QB_NEW_EN) [grammar] ~15-~15: There might be a mistake here. (QB_NEW_EN) [grammar] ~16-~16: There might be a mistake here. (QB_NEW_EN) [grammar] ~17-~17: There might be a mistake here. (QB_NEW_EN) [grammar] ~20-~20: There might be a mistake here. (QB_NEW_EN) [grammar] ~21-~21: There might be a mistake here. (QB_NEW_EN) [grammar] ~22-~22: There might be a mistake here. (QB_NEW_EN) [grammar] ~23-~23: There might be a mistake here. (QB_NEW_EN) [grammar] ~24-~24: There might be a mistake here. (QB_NEW_EN) [grammar] ~27-~27: There might be a mistake here. (QB_NEW_EN) [grammar] ~28-~28: There might be a mistake here. (QB_NEW_EN) [grammar] ~29-~29: There might be a mistake here. (QB_NEW_EN) [grammar] ~34-~34: There might be a mistake here. (QB_NEW_EN) [grammar] ~37-~37: There might be a mistake here. (QB_NEW_EN) [grammar] ~38-~38: There might be a mistake here. (QB_NEW_EN) [grammar] ~39-~39: There might be a mistake here. (QB_NEW_EN) [grammar] ~40-~40: There might be a mistake here. (QB_NEW_EN) [grammar] ~41-~41: There might be a mistake here. (QB_NEW_EN) [grammar] ~42-~42: There might be a mistake here. (QB_NEW_EN) [grammar] ~45-~45: There might be a mistake here. (QB_NEW_EN) [grammar] ~46-~46: There might be a mistake here. (QB_NEW_EN) [grammar] ~47-~47: There might be a mistake here. (QB_NEW_EN) [grammar] ~48-~48: There might be a mistake here. (QB_NEW_EN) [grammar] ~51-~51: There might be a mistake here. (QB_NEW_EN) [grammar] ~52-~52: There might be a mistake here. (QB_NEW_EN) [grammar] ~53-~53: There might be a mistake here. (QB_NEW_EN) [grammar] ~54-~54: There might be a mistake here. (QB_NEW_EN) [grammar] ~55-~55: There might be a mistake here. (QB_NEW_EN) [grammar] ~58-~58: There might be a mistake here. (QB_NEW_EN) [grammar] ~59-~59: There might be a mistake here. (QB_NEW_EN) [grammar] ~60-~60: There might be a mistake here. (QB_NEW_EN) [grammar] ~65-~65: There might be a mistake here. (QB_NEW_EN) [grammar] ~66-~66: There might be a mistake here. (QB_NEW_EN) [grammar] ~67-~67: There might be a mistake here. (QB_NEW_EN) [grammar] ~68-~68: There might be a mistake here. (QB_NEW_EN) [grammar] ~69-~69: There might be a mistake here. (QB_NEW_EN) [grammar] ~72-~72: There might be a mistake here. (QB_NEW_EN) [grammar] ~73-~73: There might be a mistake here. (QB_NEW_EN) [grammar] ~74-~74: There might be a mistake here. (QB_NEW_EN) [grammar] ~75-~75: There might be a mistake here. (QB_NEW_EN) [grammar] ~76-~76: There might be a mistake here. (QB_NEW_EN) [grammar] ~81-~81: There might be a mistake here. (QB_NEW_EN) [grammar] ~93-~93: There might be a mistake here. (QB_NEW_EN) [grammar] ~94-~94: There might be a mistake here. (QB_NEW_EN) [grammar] ~102-~102: There might be a mistake here. (QB_NEW_EN) [style] ~103-~103: Consider shortening or rephrasing this to strengthen your wording. (MAKE_CHANGES) [grammar] ~104-~104: There might be a mistake here. (QB_NEW_EN) [grammar] ~105-~105: There might be a mistake here. (QB_NEW_EN) [grammar] ~106-~106: There might be a mistake here. (QB_NEW_EN) .claude/commands/ci.md[style] ~13-~13: Consider using a different verb for a more formal wording. (FIX_RESOLVE) [grammar] ~15-~15: There might be a mistake here. (QB_NEW_EN) [grammar] ~16-~16: There might be a mistake here. (QB_NEW_EN) [grammar] ~19-~19: There might be a mistake here. (QB_NEW_EN) [grammar] ~30-~30: There might be a mistake here. (QB_NEW_EN) [grammar] ~33-~33: There might be a mistake here. (QB_NEW_EN) [grammar] ~34-~34: There might be a mistake here. (QB_NEW_EN) [grammar] ~35-~35: There might be a mistake here. (QB_NEW_EN) [grammar] ~36-~36: There might be a mistake here. (QB_NEW_EN) [grammar] ~37-~37: There might be a mistake here. (QB_NEW_EN) [grammar] ~38-~38: There might be a mistake here. (QB_NEW_EN) .claude/commands/infrastructure.md[grammar] ~12-~12: There might be a mistake here. (QB_NEW_EN) [grammar] ~13-~13: There might be a mistake here. (QB_NEW_EN) maskfile.md[grammar] ~127-~127: There might be a mistake here. (QB_NEW_EN) [grammar] ~129-~129: There might be a mistake here. (QB_NEW_EN) [grammar] ~131-~131: There might be a mistake here. (QB_NEW_EN) [grammar] ~351-~351: There might be a mistake here. (QB_NEW_EN) [grammar] ~353-~353: There might be a mistake here. (QB_NEW_EN) [grammar] ~483-~483: There might be a mistake here. (QB_NEW_EN) [grammar] ~514-~514: There might be a mistake here. (QB_NEW_EN) [grammar] ~529-~529: There might be a mistake here. (QB_NEW_EN) [grammar] ~532-~532: There might be a mistake here. (QB_NEW_EN) [grammar] ~534-~534: There might be a mistake here. (QB_NEW_EN) [grammar] ~542-~542: There might be a mistake here. (QB_NEW_EN) [grammar] ~550-~550: There might be a mistake here. (QB_NEW_EN) [grammar] ~557-~557: There might be a mistake here. (QB_NEW_EN) [grammar] ~558-~558: There might be a mistake here. (QB_NEW_EN) [grammar] ~559-~559: There might be a mistake here. (QB_NEW_EN) [grammar] ~624-~624: There might be a mistake here. (QB_NEW_EN) [grammar] ~625-~625: There might be a mistake here. (QB_NEW_EN) [grammar] ~626-~626: There might be a mistake here. (QB_NEW_EN) [grammar] ~643-~643: There might be a mistake here. (QB_NEW_EN) [grammar] ~702-~702: There might be a mistake here. (QB_NEW_EN) [grammar] ~770-~770: There might be a mistake here. (QB_NEW_EN) [grammar] ~775-~775: There might be a mistake here. (QB_NEW_EN) [grammar] ~803-~803: There might be a mistake here. (QB_NEW_EN) [grammar] ~820-~820: There might be a mistake here. (QB_NEW_EN) [grammar] ~841-~841: There might be a mistake here. (QB_NEW_EN) [grammar] ~871-~871: There might be a mistake here. (QB_NEW_EN) [grammar] ~894-~894: There might be a mistake here. (QB_NEW_EN) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
WalkthroughAdds Claude command docs (CI, infrastructure, lint), updates Claude permissions, introduces maskfile with end-to-end workflows, adds FastAPI services (datamanager, portfoliomanager) with Dockerfiles and a Swarm stack, adjusts Pulumi security groups and Grafana secret usage, tweaks flox env, and performs minor lint-only edits in libraries. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User/Client
participant PM as Portfoliomanager (FastAPI)
participant DM as Datamanager (FastAPI)
U->>PM: GET /datamanager/health
PM->>DM: GET {DATAMANAGER_URL}/portfolio-check (timeout 5s)
alt DM returns 200
DM-->>PM: 200 OK
PM-->>U: 200 OK {"datamanager_status":"healthy","status_code":200}
else Non-200
DM-->>PM: Non-200
PM-->>U: 200 OK {"datamanager_status":"unhealthy","status_code":<code>}
end
opt Network/Error
PM--xU: 503 Service Unavailable (HTTPException)
end
sequenceDiagram
autonumber
actor Dev as Developer
participant C as Claude /ci Command
participant M as mask ci
participant Steps as Format/Lint/Test/Build
Dev->>C: /ci
C->>M: Run CI pipeline
loop CI Steps
M->>Steps: Execute next step
alt Step fails
Steps-->>M: Error output
M-->>C: Failure details
C->>C: Analyze root cause
C->>M: Apply fix + retry step
M->>Steps: Re-run failed step
else Step passes
Steps-->>M: Success
end
end
M-->>C: All steps passed
C-->>Dev: Report final CI status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Pull Request Overview
This pull request implements a complete infrastructure and application deployment overhaul, transitioning from Kubernetes/EKS to Docker Swarm with simplified local/production deployment through a unified task runner interface.
- Replaces complex Kubernetes infrastructure with lightweight Docker Swarm deployment
- Introduces task-based development workflow using maskfile commands
- Establishes two-service communication between DataManager (port 8080) and PortfolioManager (port 8081)
Reviewed Changes
Copilot reviewed 43 out of 48 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Adds S105 exception for infrastructure secrets |
maskfile.md |
Comprehensive task runner with infrastructure deployment, testing, and management commands |
libraries/python/src/internal/*.py |
Code cleanup removing commented lines and obsolete TODOs |
infrastructure/__main__.py |
Complete rewrite from EKS-based to Lightsail/Docker Swarm infrastructure |
infrastructure/*.py (deleted) |
Removes complex Kubernetes, VPC, and API Gateway configurations |
infrastructure/stack.yml |
Docker Swarm infrastructure services (Traefik, Prometheus, Grafana) |
applications/*/ |
New FastAPI-based microservices with Dockerfiles and inter-service communication |
Comments suppressed due to low confidence (1)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
chrisaddy
force-pushed
the
simple-services
branch
2 times, most recently
from
20659a0 to
dd3fc74
Compare
Graphite Automations"Assign author to pull request" took an action on this PR • (08/27/25)1 assignee was added to this PR based on John Forstmeier's automation. |
chrisaddy
force-pushed
the
simple-services
branch
from
dd3fc74 to
c267986
Compare
forstmeier
left a comment
forstmeier
left a comment
There was a problem hiding this comment.
Mostly a couple minor requests/double-checks on stuff. Plus let me know what your plan for Mask is.
| [tool.uv] | ||
| package = true | ||
| src = ["src"] | ||
|
|
||
| [tool.uv.sources] | ||
| internal = { workspace = true } |
There was a problem hiding this comment.
I wasn't able to get unit tests to run on applications/ stuff without these.
There was a problem hiding this comment.
Is this going to replace Mise? Only nitpicks are I'm not a fan of the emojis and I think there's some unnecessary stuff (e.g. "ACME") in there. I didn't do a deep review of this but I know we looked at Mask a while ago.
There was a problem hiding this comment.
Also, if we’re replacing Mise all of those resources should be gutted. I’m fine swapping a tool so long as it doesn’t slow me down if you’re offline for a while.
There was a problem hiding this comment.
removed emojis and ACME stuff
difference between mise and mask should be minimal, everything is under mask --help
chrisaddy
force-pushed
the
simple-services
branch
from
c267986 to
a00c7c5
Compare
This pull request introduces major improvements to the development workflow, service architecture, and infrastructure security for the project. The most significant changes include new documentation for development commands and architecture, conversion of the
datamanagerandportfoliomanagerservices to FastAPI microservices with Docker deployment, enhanced Docker Swarm stack configuration, and improved infrastructure security through configurable network CIDRs. Additionally, permissions and settings for development tools have been updated for better automation and CI/CD support.Why we moved from mise to mask
We migrated our task runner from mise
to mask
for a few key reasons:
Markdown as the source of truth
mask tasks live directly inside a Markdown file (maskfile.md). This means our task definitions are both executable and readable as documentation. Contributors can view and understand available commands without needing to learn a separate DSL or tool-specific config format.
Better integration with AI tooling (Claude Code, etc.)
Because mask tasks are defined in plain Markdown, they work naturally with AI code assistants like Claude Code. Tasks can be surfaced, explained, or modified inline as part of a conversation, rather than being hidden away in specialized TOML/YAML config.
Standard developer workflow
Markdown is already our default format for READMEs, docs, and design notes. Using mask keeps our developer workflow consistent and lowers the barrier for new contributors—everything looks and feels like standard documentation.
Lightweight and declarative
mask keeps tasks simple, self-documenting, and avoids over-engineering. We don’t need advanced dependency management from a task runner; instead, we value clarity and interoperability.
Documentation and Workflow Improvements
CLAUDE.mdto provide comprehensive guidance for Claude Code, including development commands, architecture overview, code standards, and deployment principles.AGENTS.mdand updated its reference inCLAUDE.mdto streamline documentation. [1] [2]Microservice Architecture and Deployment
datamanagerandportfoliomanagerservices to FastAPI, updated their Dockerfiles for Python 3.12.10 and uv-based execution, and refactored theirpyproject.tomldependencies for minimal, production-ready images. [1] [2] [3] [4] [5] [6]stack.ymlfor orchestrating both services with health checks, networking, environment variables, and deployment strategies.Infrastructure Security and Configuration
infrastructure/__main__.pyto use configurable CIDRs for Swarm manager and cluster ports, reducing public exposure and improving security. [1] [2]infrastructure/stack.yml, replacing plaintext credentials with secure secret references. [1] [2]Development Tooling and Permissions
.claude/settings.local.jsonfor enhanced automation, including support for Flox, Pulumi, Docker, and SSH access..claude/commandsfor CI, lint, and infrastructure management, enabling automated fixes and streamlined workflows. [1] [2] [3]gumto Flox manifest for improved CLI interactions.datamanagerto support Docker Swarm operations and cross-service access.Minor Code Cleanup
mhsa_network.pyfor clarity.Summary by CodeRabbit